Canonicalize github urls for the resolver

In addition to canonicalizing checkout locations, this canonicalizes packages
for the resolver. This allows two dependencies with slightly different urls
pointing to the same repository to resolve to the same location and package.

Closes #104

diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs
index a9aedb7494b2d59ab737d70ab787864a95747fe7..e49bdcb2ed643216bf8886e925e9645be2830c96 100644 (file)
--- a/src/cargo/core/source.rs
+++ b/src/cargo/core/source.rs
@@ -6,6 +6,7 @@ use url::Url;
 
 use core::{Summary, Package, PackageId};
 use sources::{PathSource, GitSource};
+use sources::git;
 use util::{Config, CargoResult};
 use util::errors::human;
 
@@ -61,7 +62,7 @@ pub enum Location {
     Remote(Url),
 }
 
-#[deriving(Clone, PartialEq, Eq)]
+#[deriving(Clone, Eq)]
 pub struct SourceId {
     pub kind: SourceKind,
     pub location: Location,
@@ -110,6 +111,25 @@ impl Show for SourceId {
     }
 }
 
+// This custom implementation handles situations such as when two git sources
+// point at *almost* the same URL, but not quite, even when they actually point
+// to the same repository.
+impl PartialEq for SourceId {
+    fn eq(&self, other: &SourceId) -> bool {
+        if self.kind != other.kind { return false }
+        if self.location == other.location { return true }
+
+        match (&self.kind, &other.kind, &self.location, &other.location) {
+            (&GitKind(..), &GitKind(..),
+             &Remote(ref u1), &Remote(ref u2)) => {
+                git::canonicalize_url(u1.to_str().as_slice()) ==
+                    git::canonicalize_url(u2.to_str().as_slice())
+            }
+            _ => false,
+        }
+    }
+}
+
 impl SourceId {
     pub fn new(kind: SourceKind, location: Location) -> SourceId {
         SourceId { kind: kind, location: location }
@@ -214,3 +234,22 @@ impl Source for SourceSet {
         return Ok(ret);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{SourceId, Remote, GitKind};
+
+    #[test]
+    fn github_sources_equal() {
+        let loc = Remote(from_str("https://github.com/foo/bar").unwrap());
+        let s1 = SourceId::new(GitKind("master".to_string()), loc);
+
+        let loc = Remote(from_str("git://github.com/foo/bar").unwrap());
+        let mut s2 = SourceId::new(GitKind("master".to_string()), loc);
+
+        assert_eq!(s1, s2);
+
+        s2.kind = GitKind("foo".to_string());
+        assert!(s1 != s2);
+    }
+}

diff --git a/src/cargo/sources/git/mod.rs b/src/cargo/sources/git/mod.rs
index f44594a28f2c04816bdfd886888c7b35e905b920..f9ec6d6682c3b6c49e038a5e66250cc81ddb0e74 100644 (file)
--- a/src/cargo/sources/git/mod.rs
+++ b/src/cargo/sources/git/mod.rs
@@ -1,4 +1,4 @@
 pub use self::utils::{GitRemote,GitDatabase,GitCheckout};
-pub use self::source::{GitSource};
+pub use self::source::{GitSource, canonicalize_url};
 mod utils;
 mod source;

diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs
index 11f86c6c55c02729565c9380e46fc43be4dfb296..6a87f318c41cc25d741476f6b3b6b261c5a5d2ea 100644 (file)
--- a/src/cargo/sources/git/source.rs
+++ b/src/cargo/sources/git/source.rs
@@ -96,7 +96,7 @@ fn strip_trailing_slash<'a>(path: &'a str) -> &'a str {
 }
 
 // Some hacks and heuristics for making equivalent URLs hash the same
-fn canonicalize_url(url: &str) -> String {
+pub fn canonicalize_url(url: &str) -> String {
     let url = strip_trailing_slash(url);
 
     // HACKHACK: For github URL's specifically just lowercase
@@ -107,7 +107,12 @@ fn canonicalize_url(url: &str) -> String {
 
     let lower_url = url.chars().map(|c|c.to_lowercase()).collect::<String>();
     let url = if lower_url.as_slice().contains("github.com") {
-        lower_url
+        if lower_url.as_slice().starts_with("https") {
+            lower_url
+        } else {
+            let pos = lower_url.as_slice().find_str("://").unwrap_or(0);
+            "https".to_string() + lower_url.as_slice().slice_from(pos)
+        }
     } else {
         url.to_string()
     };
@@ -214,6 +219,13 @@ mod test {
         assert_eq!(ident1, ident2);
     }
 
+    #[test]
+    fn test_canonicalize_idents_different_protocls() {
+        let ident1 = ident(&Remote(url("https://github.com/PistonDevelopers/piston")));
+        let ident2 = ident(&Remote(url("git://github.com/PistonDevelopers/piston")));
+        assert_eq!(ident1, ident2);
+    }
+
     fn url(s: &str) -> Url {
         url::from_str(s).unwrap()
     }